serviceability: halt and resume a feed - #4307
Conversation
`FeedStatus` and the gate that reads it landed with the subscriber work, but nothing could move the status. A staked feed sat in `Pending` for life and no feed had a way to stop publishing. Two instructions and the transitions they refuse. Halt goes only from `Active`, resume only from `Halted`, and each refusal says which way it is wrong rather than sharing one error. `Pending` to `Active` is deliberately absent: that step re-reads the stake mirror, so it belongs with the code that does, and leaving it out here keeps the two apart. The feed's own builder may sign either one, which no other feed instruction allows. RFC-28 makes halt the builder's lever and doubles it as a way to rotate the upstream source without redeploying, so a builder that cannot halt its own feed cannot do either. A `FEED_AUTHORITY` or `FOUNDATION` key may sign as well, because a feed whose builder has gone quiet must still be stoppable.
There was a problem hiding this comment.
🟢 Approval recommended
The on-chain lifecycle transitions, authorization rules, instruction wiring, error mappings, and new integration tests are consistent and complete for the stated D1 scope.
Pull request overview
This pull request adds feed lifecycle controls to the DoubleZero serviceability on-chain program so an authorized signer can halt an Active feed and resume a Halted feed, unblocking status transitions now that FeedStatus gating exists.
Changes:
- Add
HaltFeed(variant 120) andResumeFeed(variant 121) instruction variants, entrypoint routing, and processors that enforce the transition table viaFeedNotHaltable(124) andFeedNotResumable(125). - Add a shared
require_feed_writerauthorization helper that allows either the feedbuilderor aFEED_AUTHORITY | FOUNDATIONsigner to perform lifecycle changes. - Add a new SVM test suite covering allowed transitions and refusal cases, including authorization-path differentiation.
File summaries
| File | Description |
|---|---|
| smartcontract/programs/doublezero-serviceability/src/processors/feed/halt.rs | New processor to transition Active -> Halted with explicit refusal error otherwise. |
| smartcontract/programs/doublezero-serviceability/src/processors/feed/resume.rs | New processor to transition Halted -> Active with explicit refusal error otherwise. |
| smartcontract/programs/doublezero-serviceability/src/processors/feed/mod.rs | Expose halt/resume modules and add require_feed_writer to support builder-or-permission authorization. |
| smartcontract/programs/doublezero-serviceability/src/instructions.rs | Add instruction variants 120/121 plus unpack/name/debug wiring. |
| smartcontract/programs/doublezero-serviceability/src/entrypoint.rs | Route HaltFeed / ResumeFeed to the new processors. |
| smartcontract/programs/doublezero-serviceability/src/error.rs | Add FeedNotHaltable/FeedNotResumable and map them to custom codes 124/125. |
| smartcontract/programs/doublezero-serviceability/tests/feed_lifecycle_test.rs | New integration tests for lifecycle transitions and refusal/authorization behavior. |
| crates/doublezero-serviceability-instruction/src/feed.rs | Add instruction builders for halt_feed and resume_feed. |
| CHANGELOG.md | Document the new instructions, transition constraints, and signer rules. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
armcconnell
left a comment
There was a problem hiding this comment.
Checked against edge-feed-spec GLOSSARY.md (a5f5cdf): conforms.
See inline comments.
|
|
||
| let mut feed = Feed::try_from(feed_account)?; | ||
| let globalstate = GlobalState::try_from(globalstate_account)?; | ||
| require_feed_writer( |
There was a problem hiding this comment.
[Important] Resume is gated on the same key set as halt, so a builder can immediately undo a FEED_AUTHORITY/FOUNDATION halt; with Retired unreachable and DeleteFeed refusing staked feeds, nothing then stops a live staked feed.
There was a problem hiding this comment.
You are right, and this was a hole rather than a nit. Fixed in d0a6b09c9.
I had given the builder co-equal power on resume, which meant an operator's halt bought nothing: the builder reverses it in the next transaction. And as you say, there is no second lever. Retired is unreachable until D2, and DeleteFeed refuses a staked feed. So the halt was the only way to stop a staked feed, and it did not stop one.
Feed now carries halted_by, set to the signer that halted:
- Halted by anyone other than the feed's builder: only a
FEED_AUTHORITYorFOUNDATIONkey can resume. - Halted by the builder: either the builder or an operator can resume, which keeps the source-rotation workflow the instruction exists for.
A pubkey rather than a flag, so an operator can see who did it, and so D2 has it. It appends to the account and reads as zero on a feed written before the field, which is right: such a feed cannot have been halted.
Two tests, and neither passes without the change: a builder is refused with NotAllowed on an operator's halt and the status stays Halted, and the builder lifts its own halt successfully.
| /// `builder` may sign this one, so a builder can rotate its upstream source without an operator. | ||
| /// A `FEED_AUTHORITY` or `FOUNDATION` key can sign it too, which is why this stays on the | ||
| /// permission-appending path. | ||
| pub fn halt_feed(program_id: &Pubkey, payer: &Pubkey, feed: &Pubkey) -> Instruction { |
There was a problem hiding this comment.
[Minor] halt_feed and resume_feed have no test; test_feed_pubkey_verbs pins the tag byte and account metas for update and delete but not for 120/121.
There was a problem hiding this comment.
Fixed in d0a6b09c9. test_feed_pubkey_verbs now pins 120 and 121 alongside 113 and 114, tag byte and account metas both.
Worth saying why that test earns its keep here specifically: DoubleZeroInstruction::unpack matches the leading byte by hand and ends in a catch-all, so a wrong or missing tag compiles cleanly and arrives at the program as InvalidInstructionData. That is exactly how it failed the first time I ran these tests, and the tag assertion is what would have caught it at the crate boundary instead.
| return Err(DoubleZeroError::FeedNotResumable.into()); | ||
| } | ||
|
|
||
| feed.status = FeedStatus::Active; |
There was a problem hiding this comment.
[Minor] Resume re-enters Active without re-reading the StakeMirror — the check Pending to Active is deferred to G1 for. Once G1 lands, a staked feed can halt, have its mirror corrected downward, and resume uncovered.
There was a problem hiding this comment.
Taken, and closed now rather than left for G1. Fixed in d0a6b09c9.
Resume re-reads the mirror and re-proves the cover, through a new require_stake_still_covers. It is not verify_stake_covers_rate: creation claims an unspent stake and so requires feed_key to be empty, while here the feed already holds the claim, so the mirror must name this feed and no other. What both check is the tier.
A staked feed that omits its mirror account is refused with StakeMirrorMissing rather than read as having no stake to check. That was the shape of the original bug, so it gets its own test.
I did not defer this. You are right that it is unreachable today, since a staked feed cannot reach Active and therefore cannot be halted. But G1 is sprint 3, and a known hole that waits for the feature that opens it is how holes ship. Closing it now costs one helper and two tests.
The test corrects the mirror down to StakeTier::None while the feed sits halted, then asserts resume is refused with StakeDoesNotCoverRate.
Three review comments, and the first one is a hole. Resume took the same signers as halt, so a builder undid an operator's halt the moment it landed. `Retired` is unreachable until D2 and `DeleteFeed` refuses a staked feed, so that halt is the only lever an operator has, and a builder that can reverse it leaves no lever at all. `Feed` now records `halted_by`, and a halt by anyone other than the builder takes an operator to lift. A builder's own halt is still the builder's to lift, which is the source rotation the whole instruction exists for. Resume also re-proves the stake. A mirror can be corrected downward while a feed sits halted, so resuming on the check made at creation would let a feed publish at a rate its stake no longer backs. A staked feed that omits its mirror is refused rather than read as having no stake to check. The builders had no test. They now pin the tag byte and the account metas, which is what catches a wrong variant: `unpack` matches the leading byte by hand with a catch-all, so the compiler never sees it.
armcconnell
left a comment
There was a problem hiding this comment.
Checked against edge-feed-spec GLOSSARY.md (a5f5cdf): conforms.
See inline comments.
| /// A halt by an operator is not the builder's to lift. Without this the builder resumes the | ||
| /// moment an operator halts, and with `Retired` unreachable and `DeleteFeed` refusing a staked | ||
| /// feed, nothing else stops one. | ||
| pub halted_by: Pubkey, // 32 |
There was a problem hiding this comment.
[Important] Breaks the fixture generator's exhaustive Feed literal (E0063, generate-fixtures/src/main.rs:1505); sdk-fixture-drift is red. Regenerate feed.bin and add the field to the Python and TypeScript decoders.
There was a problem hiding this comment.
Fixed in 28d9201e0. Adding a field to Feed reaches four places and the compiler only guards one of them.
- The fixture generator's exhaustive literal, which is the E0063 you saw.
feed.bin, regenerated.- The Python decoder, which read a feed that ended one field early and silently returned the old shape.
- The TypeScript decoder, same.
halted_by is appended after status, so a feed written before the field reads as halted by nobody. That is correct rather than merely convenient: such a feed cannot have been halted, so the default is the true answer rather than a fallback.
make check-fixtures passes, and both language suites pass against the regenerated fixture: 126 Python tests and 149 TypeScript.
| /// `ResumeFeed` (variant 121). Accounts: `[feed, globalstate]`. | ||
| /// | ||
| /// Puts a halted feed back to publishing. Signed by the same keys `halt_feed` accepts. | ||
| pub fn resume_feed(program_id: &Pubkey, payer: &Pubkey, feed: &Pubkey) -> Instruction { |
There was a problem hiding this comment.
[Minor] Omits the stake mirror account, so resuming a staked feed always fails StakeMirrorMissing; create_feed shares the gap.
There was a problem hiding this comment.
Right, and it made the builder unusable for exactly the feeds the instruction exists for. Fixed in 28d9201e0.
resume_feed now takes stake_mirror: Option<&Pubkey>, appended after the payer and system program where the processor looks for it, and None for a catalog feed. The test pins both shapes: without a mirror the accounts match the other feed verbs exactly, and with one it is the last account and read-only.
On create_feed sharing the gap: it does, and the Rust SDK's feed commands do not append it either, so a staked CreateFeed through either path fails StakeMirrorMissing today. That is pre-existing rather than something this PR introduces, and its callers currently pass the mirror themselves as an extra account. I have left it alone to keep this change to the lifecycle verbs, but say the word and I will fix it here or open it separately.
There was a problem hiding this comment.
Fixed here too, in cfa8376d4. One fewer review beats a tidier diff.
create_feed derives the mirror rather than taking it as a parameter, which is the difference between it and resume_feed: the create args already carry builder and stake_ref, so the mirror is a function of what the caller has already passed and asking for it separately would only add a way to disagree with itself. Resume has empty args and cannot derive it without fetching the feed, so it takes an Option.
Writable for create, since claiming the stake writes feed_key onto it. Read-only for resume, which only reads the tier. A catalog feed sends none in either, so a caller that sent neither before is unaffected.
Tested both directions: a staked create ends with the mirror as its last account, a catalog create carries no mirror at all. The program's own feed tests still pass, which is the check that matters, since they pass the mirror by hand and would disagree if the derivation were wrong.
Adding a field to `Feed` broke the fixture generator's exhaustive literal and left the Python and TypeScript decoders reading a feed that ends one field early. The Rust compiler caught the first and could not catch the other two. `halted_by` is appended after `status`, so a feed written before it reads as halted by nobody. That is right rather than merely convenient: such a feed cannot have been halted. `resume_feed` also never appended the stake mirror, so resuming a staked feed through this builder always failed `StakeMirrorMissing`. It now takes the mirror, `None` for a catalog feed, and the test pins where it lands.
`CreateFeed` reads the tier from the mirror and writes the feed's key onto it to claim the stake, and the builder never sent it, so a staked create through this crate always failed. Its callers passed the account by hand. Derived rather than taken as a parameter. Unlike `resume_feed`, the args already carry the stake the mirror is seeded on, so asking a caller for it would only add a way to get it wrong. A catalog feed sends none, so a caller that sent neither before is unaffected.
Implements D1.
FeedStatusand the gate that reads it landed with the subscriber work, but nothing could move the status: a staked feed sat inPendingfor life, and no feed had a way to stop publishing.The transition table
HaltFeedResumeFeedPendingActiveHaltedHaltedActiveRetiredEach refusal says which way it is wrong,
FeedNotHaltable(124) orFeedNotResumable(125), rather than sharing one error. A caller that halts twice should learn it did not stop something a second time, not read a generic argument failure.PendingtoActiveis deliberately absent. That step re-reads the stake mirror, which is the only thing that catches a mirror corrected after it admitted a feed, so it belongs with the code that does it (G1). Leaving it out here keeps the two apart rather than half-implementing it.Who can sign
The feed's own
buildermay sign either instruction. No other feed instruction consultsfeed.builderat all; they authorize onFEED_AUTHORITY | FOUNDATIONand stop there.Both paths are needed, for different reasons:
FEED_AUTHORITYorFOUNDATIONkey, because a feed whose builder has gone quiet must still be stoppable, and every other feed instruction already works that way.A catalog feed has no builder, so only the second path applies to it. The check compares against the default pubkey explicitly rather than relying on nobody being able to sign as it.
Testing Verification
Five tests in a new
feed_lifecycle_test.rs. A new file rather than helpers bolted ontofeed_test.rs, because itsinit_stakedhardcodes a builder the test cannot sign as, and other tests depend on that.Pendingfeed neither halts nor resumes. This also guards G1's territory: nothing here moves a feed out ofPending.Pendingfeed can be halted by anyone and G1 does not exist yet to make a staked feedActive. The errors are what distinguish them: the builder reaches the status check and getsFeedNotHaltable, the stranger does not get past authorization and getsNotAllowed.That last one is the honest shape of the test available today. A staked feed cannot reach
Activeuntil G1, so the builder's successful halt cannot be tested yet, and asserting on which check stopped the call is the strongest available evidence that the authorization path works.Full suite passes: 327 serviceability tests plus the 5 new ones, 86 in the instruction crate,
clippy --all-targets -Dwarningsandfmt --checkclean.One thing the compiler could not catch, worth knowing for the next variant:
DoubleZeroInstruction::unpackmatches the leading byte by hand with a catch-all, andFrom<ProgramError> for DoubleZeroErrormatches the code the same way. Both compile fine with a variant missing. The first showed up asInvalidInstructionDataat runtime and the second as a round-trip test failure.